[examples] Verdure client cleanup and dependency updates #646
Conversation
parlough
force-pushed
the
misc/verdure-client-cleanup
branch
from
b247ef5 to
663ddff
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request focuses on cleaning up the Verdure client example and updating dependencies. The changes include adopting modern Dart features like pattern matching, using the spacing property in Row and Column widgets for cleaner layouts, and updating Riverpod and related packages to newer versions. The code is generally improved and more maintainable. I've left a couple of minor suggestions for improvement. Please also note that the repository's style guide requires a Pre-Review Checklist in the PR description, which is currently missing.
| const SizedBox(height: 16), | ||
| Text( | ||
| '''Upload a photo of your front or back yard, and our designers will use it to create a custom vision. Get ready to see the potential.''', | ||
| 'Upload a photo of your front or back yard, ' |
There was a problem hiding this comment.
I was using triple quotes here because agents seem to have a hard time wrapping these multi-line strings, and triple quotes avoid the lint warning. I'm fine with converting them back, but it does mean more manual work if the LLMs modify the strings, since they will inevitably fail to wrap them properly.
There was a problem hiding this comment.
Ah, thanks for the context here and thanks for raising this.
I think since this is an example, we should still try to be idiomatic even if it involves a little extra work.
I do wonder what we can do to make agents better at breaking strings in general. I find while agents don't handle wrapping/splitting strings well, LLM-based tab/auto-complete handles it quite well across different editors. Perhaps similar examples in the edited file are ending up closer in the context window?
There was a problem hiding this comment.
Works for me. It think it's just that they are bad at counting characters. I think they need an edit tool that takes a line range and wraps it, contextually, based on language and context in the code.
| // Went from true to false, reset messages after a short delay | ||
| // to allow the fade-out animation to complete. | ||
| Future.delayed(const Duration(milliseconds: 500), clearMessages); | ||
| Future<void>.delayed(const Duration(milliseconds: 500), clearMessages); |
There was a problem hiding this comment.
Perhaps we should enable strict-raw-types (https://dart.dev/tools/analysis#enabling-additional-type-checks)? Or is that too much?
There was a problem hiding this comment.
I'm all for it, but since we use a workspace-wide analysis config, I can do that in a follow-up PR for the repository.
I enabled it temporarily for verdure and fixed the one other instance of it.
|
Thanks for the review @gspencergoog! I don't seem to have write/merge access on this repository, so feel free to merge when you're happy :D |
|
Sorry, I missed that step. :-) Done! |
2 participants
No description provided.